Composefs soft reboot #1828

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Open

Johan-Liebert1 wants to merge 7 commits into bootc-dev:main from Johan-Liebert1:cfs-soft-reboot

Collaborator

Johan-Liebert1 commented Dec 4, 2025 •

edited

Loading

Requires: containers/composefs-rs#198 and a bump to composefs-rs

Add an internal command for soft rebooting the system. Similar to how
it's done for ostree, we only allow soft reboot if the other deployment
has the same kernel state, i.e. the SHASum of kernel + initrd is the
same as that of the current deployment.

Similar to soft reboots for Type1 entries, we compute the SHA256Sum of
.linux + .initrd sections in the UKI, and compare them to check for
kernel skew

Next, compare the .cmdline section skipping the composefs= parameter
as that will always be different

After a soft reboot the kernel cmdline doesn't change so we can't rely
on the composefs= parameter in the cmdline. Instead, we check the
source of the root mount point

Johan-Liebert1 requested a review from cgwalters

December 4, 2025 08:36

bootc-bot bot requested a review from jmarrero

December 4, 2025 08:36

gemini-code-assist bot reviewed

View reviewed changes

Contributor

gemini-code-assist bot left a comment

Code Review

This pull request introduces support for soft reboots for composefs deployments, a significant feature that aligns its capabilities more closely with ostree. The changes include a new internal command for preparing the soft reboot, logic to determine soft reboot capability based on kernel state, and an updated mechanism for detecting the booted deployment after a soft reboot by inspecting the root mount source.

Overall, the implementation is well-thought-out. However, I've identified a couple of critical issues. One is an error handling bug where a failure in the exec call for systemctl soft-reboot would be silently ignored. Another is a logic error in the usage of OnceLock which would fail to detect a soft reboot correctly on subsequent calls. I've also provided a suggestion to refactor a section of the code to improve its conciseness and readability. Addressing these points will enhance the robustness and maintainability of this new feature.

crates/lib/src/bootc_composefs/soft_reboot.rs Outdated Show resolved Hide resolved

crates/lib/src/bootc_composefs/status.rs Show resolved Hide resolved

crates/lib/src/bootc_composefs/status.rs Outdated Show resolved Hide resolved

Johan-Liebert1 force-pushed the cfs-soft-reboot branch from 45d51d9 to a1a0e0f Compare

December 4, 2025 09:19

Collaborator Author

Johan-Liebert1 commented Dec 4, 2025

plan-20-image-pushpull-upgrade seems to have failed with

content: Failed to kill unit test-cat-progress.service: Unit test-cat-progress.service not loaded.
content: Running as unit: test-cat-progress.service; invocation ID: ad2c2ea1c85a4253a2fc81619b9ade72
content: layers already present: 65; layers needed: 11 (267.8 MB)
content: Deploying...done (5 seconds)
content: Bound images stored: 3
content: Queued for next boot: ostree-unverified-image:containers-storage:localhost/bootc-derived
content:   Version: 10
content:   Digest: sha256:38e79b454a170138c58e2add8b6106ff374d63c03c33f8ef693590f0e28adf17
content: -- No entries --
content: Adding quoted karg via rpm-ostree to test ostree issue #3544
content: error: System transaction in progress
content: Connection to localhost closed.

Is this related to rpm-ostree? coreos/rpm-ostree#210 seems related, but not sure

cgwalters reviewed

View reviewed changes

crates/lib/src/bootc_composefs/status.rs Outdated Show resolved Hide resolved

crates/lib/src/bootc_composefs/status.rs

Comment on lines +91 to +93

    
                  let verity_from_mount_src = root_mnt

                      .source

                      .strip_prefix("composefs:")

                      .ok_or_else(|| anyhow::anyhow!("Root not mounted using composefs"))?;

Collaborator

cgwalters Dec 4, 2025

Hum...I'm OK with this but ultimately I think it'd be even nicer to have truly structured metadata associated with the mount point somehow.

Collaborator Author

Johan-Liebert1 Dec 5, 2025

Wouldn't the fsconfig string be that metadata?

crates/lib/src/bootc_composefs/status.rs

    
                  // This is of the format composefs:<composefs_hash>

                  let verity_from_mount_src = root_mnt

                      .source

                      .strip_prefix("composefs:")

Collaborator

cgwalters Dec 4, 2025

Can we strongly associate this string with the thing that creates it via a const?

Collaborator Author

Johan-Liebert1 Dec 5, 2025 •

edited

Loading

I wanted to, but it's done in composefs-rs. Maybe we can export it from there

crates/lib/src/bootc_composefs/status.rs Outdated

    
                      host.spec.boot_order = BootOrder::Rollback

                  };

                  // Can only soft reboot non UKI boot entries

Collaborator

cgwalters Dec 4, 2025

Why?

Collaborator Author

Johan-Liebert1 Dec 5, 2025

It is a little bit more complex, as we'd have to extract .linux and .initrd sections to compute the hash. Definitely possible though. That being said, in the confidential clusters use case, would soft rebooting be a thing anyway?

Collaborator

cgwalters Dec 8, 2025

We want to support sealed systems outside of just confidential workloads.

It is a little bit more complex, as we'd have to extract .linux and .initrd sections to compute the hash.

There are other UKI components too - such as the critical .cmdline. I think what we really need to do here is a complete diff the two, just dropping out composefs= from the .cmdline or so.

Collaborator

cgwalters commented Dec 4, 2025

content: error: System transaction in progress

It's a flake (well, a bug somewhere), I'll ensure we have an issue for it

Johan-Liebert1 force-pushed the cfs-soft-reboot branch from a1a0e0f to a4a3840 Compare

December 5, 2025 10:49

This comment was marked as outdated.

Sign in to view

cgwalters added the needs-rebase label

Collaborator

cgwalters commented Dec 9, 2025

Hmm...right I think we need to use pull_request_target for the rebase helper

Collaborator Author

Johan-Liebert1 commented Dec 10, 2025

I have rebased it locally. Working on soft-reboot for UKIs

Johan-Liebert1 added 7 commits

December 11, 2025 12:26


          composefs/status: Check if deployment is soft rebootable

e9d4cf1

Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>


          initramfs: Allow passing target for mounting

debe49f

`target` field in Args was not being used. Use it if it is passed in the
args. Also helps us mount the new root at `/run/nextroot`

Also, use Cmdline struct instead of String to represent the kernel
command line

Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>


          composefs: Implement soft reboot

e4ebf1c

Add an internal command for soft rebooting the system. Similar to how
it's done for ostree, we only allow soft reboot if the other deployment
has the same kernel state, i.e. the SHASum of kernel + initrd is the
same as that of the current deployment.

soft reboot is not possible in case of UKI deployment

Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>


          composefs: Handle bootc status after a soft reboot

5f98c9e

After a soft reboot the kernel cmdline doesn't change so we can't rely
on the `composefs=` parameter in the cmdline. Instead, we check the
source of the root mount point

Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>


          composefs: Soft Reboot after update/switch if specified

Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>


          composefs/uki: Save boot digest

e0f64c5

Similar to what we do with Type1 entries, we save the SHA256Sum of
.linux + .initrd sections of the UKI under `boot_digest` key in the
origin file

Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>


          composefs/soft-reboot: Handle soft reboot for UKIs

33c84f1

Similar to soft reboots for Type1 entries, we compute the SHA256Sum of
.linux + .initrd sections in the UKI, and compare them to check for
kernel skew

Next, compare the .cmdline section skipping the `composefs=` parameter
as that will always be different

Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>

Johan-Liebert1 force-pushed the cfs-soft-reboot branch from a4a3840 to 33c84f1 Compare

December 11, 2025 06:56

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels